Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

🌱 initial workflow run collect data #194

Merged
merged 12 commits into from
Jan 29, 2025
Merged

🌱 initial workflow run collect data #194

merged 12 commits into from
Jan 29, 2025

Conversation

savitharaghunathan
Copy link
Member

@savitharaghunathan savitharaghunathan commented Jan 13, 2025 •
edited
Loading

initial workflow run collect data

To run workflow mode,

export GITHUB_TOKEN="your_token"
export WORKFLOW=True

run npm run collect-assets

for release mode,

export WORKFLOW=False

run npm run collect-assets

@savitharaghunathan
initial workflow run collect data
787faa7 
Signed-off-by: Savitha Raghunathan <[email protected]>
@savitharaghunathan savitharaghunathan requested a review from a team as a code owner January 13, 2025 22:00
@savitharaghunathan savitharaghunathan marked this pull request as draft January 13, 2025 22:00
@savitharaghunathan savitharaghunathan self-assigned this Jan 13, 2025
sjd78
sjd78 requested changes Jan 16, 2025
View reviewed changes
Copy link
Member

@sjd78 sjd78 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of thought mostly because I was thinking through a bunch of things as I was reviewing. The base is solid, mostly just needs some reorganization.

scripts/_util.js Outdated Show resolved Hide resolved
scripts/_util.js Outdated Show resolved Hide resolved
scripts/_util.js Outdated Show resolved Hide resolved
scripts/_util.js Outdated Show resolved Hide resolved
scripts/collect-assets-workflow.js Outdated Show resolved Hide resolved
scripts/_download.js Outdated Show resolved Hide resolved
scripts/_download.js Outdated Show resolved Hide resolved
scripts/_download.js Outdated Show resolved Hide resolved
scripts/_download.js Outdated Show resolved Hide resolved
scripts/collect-assets-workflow.js Outdated Show resolved Hide resolved
@savitharaghunathan
adding review fedback
0907f88 
Signed-off-by: Savitha Raghunathan <[email protected]>
@savitharaghunathan savitharaghunathan marked this pull request as ready for review January 16, 2025 19:40
@savitharaghunathan
use env variables to select workflow/release
072719c 
Signed-off-by: Savitha Raghunathan <[email protected]>
@savitharaghunathan
setting exe bit
81f4c51 
Signed-off-by: Savitha Raghunathan <[email protected]>
@savitharaghunathan
Copy link
Member Author

issue: #198

@sjd78 sjd78 linked an issue Jan 21, 2025 that may be closed by this pull request
Update collect-assets to download kai assets from main branch / latest CI workflows #198
Closed
djzager
djzager requested changes Jan 21, 2025
View reviewed changes
Copy link
Member

@djzager djzager left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only one thing I notice

scripts/collect-assets.js Show resolved Hide resolved
djzager
djzager previously approved these changes Jan 21, 2025
View reviewed changes
Copy link
Member

@djzager djzager left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

This was referenced Jan 21, 2025
Closed
Open
Open
@savitharaghunathan
Copy link
Member Author

@sjd78 updated error handling to exit the process if there is an error. ptal

npm run collect-assets

> [email protected] collect-assets
> rimraf ./downloaded_assets && node ./scripts/collect-assets.js

working from: /Users/sraghuna/local_dev/editor-extensions
WORKFLOW environment variable is not set. Downloading GitHub release assets...
Fetching GitHub release metadata for konveyor/kai release: v0.0.5
Error during asset collection: Error in downloadGitHubRelease: Error in getGitHubReleaseMetadata: Failed to fetch release metadata: Not Found

@savitharaghunathan
update error handling
d06c02c 
Signed-off-by: Savitha Raghunathan <[email protected]>
@savitharaghunathan
Copy link
Member Author

fixes #235

@savitharaghunathan savitharaghunathan linked an issue Jan 21, 2025 that may be closed by this pull request
Update collect-assets to fail if target release tag cannot be found #235
Open
savitharaghunathan and others added 2 commits January 21, 2025 15:23
@savitharaghunathan
reset download tag to 0.0.4
29c9142 
Signed-off-by: Savitha Raghunathan <[email protected]>
@savitharaghunathan
Merge branch 'main' into workflow_artifacts
73387aa 
Signed-off-by: Savitha Raghunathan <[email protected]>
sjd78
sjd78 requested changes Jan 28, 2025
View reviewed changes
Copy link
Member

@sjd78 sjd78 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few initial comments. I'll check on my shoulds and update as needed.

package.json Outdated Show resolved Hide resolved
scripts/_download.js Outdated Show resolved Hide resolved
scripts/_download.js Outdated Show resolved Hide resolved
scripts/_download.js Outdated Show resolved Hide resolved
scripts/collect-assets.js Outdated Show resolved Hide resolved
scripts/collect-assets.js Outdated Show resolved Hide resolved
@sjd78
Copy link
Member

sjd78 commented Jan 28, 2025

fixes #235

I'd rather have this work done in another PR so it is easier to review those change apart from the changes to pull assets from workflow artifacts.

@savitharaghunathan
Copy link
Member Author

savitharaghunathan commented Jan 28, 2025 •
edited
Loading

@sjd78 I am gonna remove the try/catch blocks because we want to tackle #235 in a different PR

sjd78
sjd78 reviewed Jan 28, 2025
View reviewed changes
scripts/_util.js Outdated Show resolved Hide resolved
@savitharaghunathan
remove debug statements
7e9cbf0 
Signed-off-by: Savitha Raghunathan <[email protected]>
@savitharaghunathan savitharaghunathan removed a link to an issue Jan 28, 2025
Update collect-assets to fail if target release tag cannot be found #235
Open
@sjd78 sjd78 mentioned this pull request Jan 20, 2025
Open
sjd78
sjd78 requested changes Jan 29, 2025
View reviewed changes
Copy link
Member

@sjd78 sjd78 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The download and extract process is working, but the target extracted location doesn't line up with the extension needs.

We could merge this PR and then followup with 2 PRs:

Then we can add the workflow artifact download to the ci builds....

scripts/_download.js Show resolved Hide resolved
@savitharaghunathan
Copy link
Member Author

@sjd78 we can merge this and I can follow up the fixes in the next pr

sjd78
sjd78 approved these changes Jan 29, 2025
View reviewed changes
Copy link
Member

@sjd78 sjd78 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Download works well. Some issues to be solved in followup before integration to the ci actions.

@sjd78 sjd78 modified the milestone: v0.1.0 Jan 29, 2025
@savitharaghunathan savitharaghunathan merged commit 3c6f653 into konveyor:main Jan 29, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rszwajko rszwajko rszwajko left review comments

@sjd78 sjd78 sjd78 approved these changes

@djzager djzager djzager left review comments

Assignees

@savitharaghunathan savitharaghunathan

Labels
None yet
Projects
Kai Tech Preview Release
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update collect-assets to download kai assets from main branch / latest CI workflows
4 participants
@savitharaghunathan @sjd78 @djzager @rszwajko